Fix generate to be correctly implemented range.#4528
Fix generate to be correctly implemented range.#4528dnadlinger merged 2 commits intodlang:masterfrom
Conversation
|
This somewhat depends on #4511 |
| $(D popFront), and the cached value returned when $(D front) is called. | ||
|
|
||
| Params: | ||
| fun = is the $(D isCallable) that gets called on every call to front. |
There was a problem hiding this comment.
The Params: section is missing now?
There was a problem hiding this comment.
Not needed, basically said the same thing as Returns section.
|
Can someone explain to me the failure in the travis build? |
|
Note: I'm working on an update to changelog... |
| import std.traits: ReturnType, functionAttributes, FunctionAttribute; | ||
| enum returnByRef_ = (functionAttributes!fun & FunctionAttribute.ref_) ? true : false; | ||
| static if(returnByRef_) | ||
| ReturnType!fun *elem_; |
There was a problem hiding this comment.
Space between required if (. This is why Travis complains.
Likewise, at least one if and one foreach below.
There was a problem hiding this comment.
OK. that is so not clear from the error message 😄
Yeah, I wouldn't pull this until that's settled. We still haven't heard Andrei's opinion. |
I think Andrei and Walter are on the same page for that. Andrei seems stretched thin, so it may be a while. |
|
What about a unittest that verifies that multiple calls to |
|
I used generate in solving a Project Euler problem and switching the new implementation in still gave the right answer, so LGTM. |
|
@quickfur added |
| ------- | ||
| ) | ||
|
|
||
| $(LI $(LNAME2 generate, `std.range.generate` fixed to be a proper range.) |
There was a problem hiding this comment.
I would add a note here about following the clarified range rules with a link, when #4511 is pulled
There was a problem hiding this comment.
Most expect ranges to behave this way, the updating of the rules is just a formal presentation of what everyone already knows. So I don't think a link is necessary.
|
LGTM. |
|
ping, #4511 has been merged. |
std/range/package.d
Outdated
| foreach loop. | ||
| A by-value foreach loop means that the loop value is not $(D ref). | ||
| The resulting range will call $(D fun()) on construction, and every call to | ||
| $(D popFront), and the cached value returned when $(D front) is called. |
There was a problem hiding this comment.
"is/will be returned"?
There was a problem hiding this comment.
yep, this grammar error "will be" fixed...
|
nits fixed. |
std/range/package.d
Outdated
|
|
||
| Returns: an $(D inputRange) that returns a new value generated by $(D Fun) on | ||
| any call to $(D front). | ||
| Returns: an $(D inputRange) where each element represents another call to fun. |
There was a problem hiding this comment.
backticks over $(D ) throughout
|
LGTM sans comments |
|
Done. |
|
I meant backticks in your changes, but w/e :) |
|
My OCD forbade that idea :) |
|
travis fails |
Restarted. I fixed that issue already in another PR I think. |
|
Please squash the relevant parts; I'll merge then. |
|
@klickverbot done |
|
Auto-merge toggled on |
Current coverage is 88.68% (diff: 100%)@@ master #4528 diff @@
==========================================
Files 121 121
Lines 73827 73850 +23
Methods 0 0
Messages 0 0
Branches 0 0
==========================================
+ Hits 65471 65494 +23
Misses 8356 8356
Partials 0 0
|
8 participants
A bit more complicated to deal with the possibility of ref returns. Also added a unit test to ensure ref returns work.
See rationale here: https://forum.dlang.org/post/nipa43$k0i$1@digitalmars.com